Match requested dependencies in ModuleHasDependency and RepositoryHasDependency#179
Merged
Merged
Conversation
…Dependency After #176 removed DependencyInsight, both recipes only inspect resolved dependencies. Add a fallback that also matches against requested (declared) dependencies, so a dep that is declared but unresolved (e.g. unit tests with no repositories block, partially resolved POMs) is still recognized. Mirrors the existing behavior of `FindDependency` (both Maven and Gradle), which matches on what is declared in the build file rather than on the resolved tree. This keeps the two families of "find" recipes consistent and unblocks a 1-on-1 refactor that swaps one for the other in cleaner recipes. Version glob handling: null version is accepted (no constraint declared); `${...}` placeholders are skipped (cannot be evaluated); otherwise compared against the version pattern.
github-project-automation
Bot
moved this from In Progress
to Ready to Review
in OpenRewrite
timtebeek
added a commit
that referenced
this pull request
May 29, 2026
* Trust resolved version over declared dep in dependency search PR #179 extended ModuleHasDependency / RepositoryHasDependency to also match against requested/declared dependencies so they still match when resolution fails. This introduced false positives whenever a version range is supplied: the declared version string could satisfy the comparator even when the resolved version did not (e.g. a Gradle resolutionStrategy.force or platform alignment overriding the declared coordinate). - When a coordinate is present in the resolved dependencies, trust the resolved check and skip the declared-dependency fallback for that group:artifact. Only fall back for declared deps not already resolved. - Tighten versionMatches so a null declared version no longer auto-matches when a version constraint is supplied (same treatment as a ${...} property reference). Adds regression tests covering both rules in ModuleHasDependencyTest and RepositoryHasDependencyTest. * Apply suggestions from code review Co-authored-by: Jente Sondervorst <jentesondervorst@gmail.com> * Add Maven regression tests for BOM-managed dependency version range --------- Co-authored-by: Jente Sondervorst <jentesondervorst@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DependencyInsight,ModuleHasDependencyandRepositoryHasDependencyonly inspect resolved dependencies. A dependency that is declared but never resolved (unit tests without arepositoriesblock, partially resolved POMs) is now invisible to these recipes.FindDependencyfor both Maven (walks the pom XML tags) and Gradle (usesGradleDependency.Matcheragainst the build script AST) — both already match declared dependencies, not the resolved tree. BringingModuleHasDependency/RepositoryHasDependencyin line keeps the "find" family of recipes consistent and unblocks a 1-on-1 refactor that swaps one for the other in cleaner recipes.Version glob handling on the requested-deps path:
null→ accept (no constraint declared).${→ skip (placeholder; cannot evaluate).Scope filtering is applied to Maven requested deps (default
compilewhen null); Gradle does not currently apply scope filtering on the resolved path either, so the requested path mirrors that.Test plan
gradleMatchesOnRequested— Gradle build with norepositoriesblock; declaredspring-beansmatched via requested fallback.mavenMatchesOnRequested— pom with no<repositories>, pinned emptyMavenSettings(same trick as Pin empty MavenSettings in DependencyResolutionDiagnosticTest.maven() #178) so resolution actually fails; declaredspring-beansmatched via requested fallback.gradleVersionRangeOnRequestedDoesNotMatchWhenOutOfRange— version glob is still enforced on the requested-deps path.ModuleHasDependencyTest+RepositoryHasDependencyTestsuites still pass.